Skip to content

gh-140036: _pydecimal: avoid slow exponentiation in floor division - #140044

Merged
picnixz merged 11 commits into
python:mainfrom
picnixz:fix/decimal/infinite-prec-140036
Oct 23, 2026
Merged

picnixz merged 11 commits into
python:mainfrom
picnixz:fix/decimal/infinite-prec-140036

Conversation

@picnixz

@picnixz picnixz commented Oct 13, 2025 •

Copy link
Copy Markdown
Member

For now, we'll only fix this code path and we'll need to discuss a bit more for the other @mdickinson reported (#140036 (comment)).

@picnixz

picnixz commented Oct 13, 2025

Copy link
Copy Markdown
Member Author

Note: I didn't add tests yet because I was too lazy. I'll do it once we agree on the code.

Comment thread Lib/_pydecimal.py Outdated
Comment on lines +1360 to +1362
if q.bit_length() < 1 + context.prec * _LOG_10_BASE_2:
# ensure that the previous check was sufficient
if len(str_q := str(q)) <= context.prec:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think both variants are fine, but you should choose one. You shouldn't worry about string conversion limit.

@picnixz picnixz Oct 13, 2025 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was to prevent corner cases. I don't have the time to check, but I feel that it's possible to have some q such that q.bit_length() < 1 + context.prec * _LOG_10_BASE_2 is true but len(str(q)) <= context.prec is false. But maybe this is impossible.

I also didn't want to compute str(q) before as it could be an expensive check. If if q.bit_length() < 1 + context.prec * _LOG_10_BASE_2 already fails, there is no need to compute str(q) which could also raise an exception.

@efimov-mikhail efimov-mikhail Oct 13, 2025 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was to prevent corner cases. I don't have the time to check, but I feel that it's possible to have some q such that q.bit_length() < 1 + context.prec * _LOG_10_BASE_2 is true but len(str(q)) <= context.prec is false. But maybe this is impossible.

It's impossible if _LOG_10_BASE_2 is less or equal than real mathematical value of log_2(10) and we change condition to q.bit_length() < context.prec * _LOG_10_BASE_2.

Let's prove it:
a) q.bit_length() < context.prec * _LOG_10_BASE_2 is true
BUT
b) len(str(q)) <= context.prec is false.

We have
a) q.bit_length() < context.prec * _LOG_10_BASE_2
b) len(str(q)) > context.prec.
Since len(str(q)) and context.prec are integers, we have len(str(q)) >= context.prec + 1,
and q >= 10 ** context.prec.
Then we apply mathematical log_2:
log_2 (q) >= context.prec * log_2(10), when log_2(10) is exact value.

Since log_2(10) >= _LOG_10_BASE_2, we have log_2 (q) >= context.prec * log_2(10) >= context.prec * _LOG_10_BASE_2 > q.bit_length().
As a result, q > 2 ** q.bit_length()
And this is impossible inequation.

It seems that checking of q.bit_length() < context.prec * _LOG_10_BASE_2 would be good enough.
If it doesn't, construction of str looks like best solution.

And some small example:

>>> q = 110
>>> prec = 2
>>> q.bit_length() < 1 + prec * math.log2(10)
True
>>> len(str(q)) <= prec
False

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the analysis.

It's impossible if _LOG_10_BASE_2 is less or equal than real mathematical value of log_2(10)

This appears to be the case as pow(2, float.fromhex('0x1.a934f0979a371p+1')) is something like 9.99...98.

And some small example:

Ok, so the 1+ was indeed too much (that's what I feared). This will also help in changing the q > 10 ** prec case as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Michail, thanks for a correction of the first inequality q.bit_length() < context.prec * _LOG_10_BASE_2 (1).

Unfortunately, I don't think that this inequality could be used as an "optimization". Remember, it should be an equivalent of q < 10**context.prec (2). I.e. both inequalities should have same boolean values. In particular, if (1) is false - (2) should be false, as in this case we miss verification by the second check.

This equivalency is trivial for len(str(q)) <= context.prec (3) and (2), where q >= 0 and context.prec > 0 are integers. I think we should use this, there is no possible short-cut.

@efimov-mikhail efimov-mikhail Oct 14, 2025 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's impossible to make equivalent inequality only with q.bit_length. But we can significantly reduce the needs of calculation of len(str(q)). We already prove that if q.bit_length() < context.prec * _LOG_10_BASE_2 then len(str(q)) <= context.prec and there's no need to check. On the other hand, if q.bit_length() >= 1 + context.prec * _LOG_10_BASE_2_G, then q >= 2**(q.bit_length()-1) >= 2**(context.prec * _LOG_10_BASE_2_G) >= 2**(context.prec * log_2(10)) = 10**context.prec, where _LOG_10_BASE_2_G = float.fromhex('0x1.a934f0979a372p+1') which is slightly greater then exact value of log_2(10).

It means we could implement checking this way:

_LOG_10_BASE_2 = float.fromhex('0x1.a934f0979a371p+1')
_LOG_10_BASE_2_G = float.fromhex('0x1.a934f0979a372p+1')

def q_is_greater_or_equal_than_pow_10_a(q: int, a: int) -> bool:
    if q.bit_length() < a * _LOG_10_BASE_2:
        return False
    elif q.bit_length() >= 1 + a * _LOG_10_BASE_2_G:
        return True
    else:
        return len(str(q)) > a

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has more sense for me. Though, IMO such helper not worth if it's required in one or two places in code.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I forgot to ask yesterday to check for the reverse condition (I started taking my pen & paper but then had to leave). I'll go for a helper as it's used more than once just for clarity purposes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's used more than once

I guess, you are planning yet for another instance in #140036 (comment). Two cases - not too much...

On another hand, we have a lot of len(str) computations in the module:

$ git grep 'len(str' Lib/_pydecimal.py | wc -l
40

IMO, keeping code simple is better. Maybe we should apply instead wishful thinking that eventually int->str conversion will be fast.

@picnixz
picnixz marked this pull request as draft October 13, 2025 16:05
@picnixz

picnixz commented Oct 13, 2025

Copy link
Copy Markdown
Member Author

I need to go for today so I'm putting it on hold (I'll change the other places).

@picnixz
picnixz force-pushed the fix/decimal/infinite-prec-140036 branch from 4951511 to 329c31a Compare October 14, 2025 10:22
@picnixz

picnixz commented Oct 14, 2025

Copy link
Copy Markdown
Member Author

So I've pushed an inlined version because in one case we need to anyway compute str_q (so I prefer forwarding the exception normally). For the other case where str_q doesn't need to be computed, I fall back to a slow path ($10^{prec}$) because otherwise it would be a behavioral change.

I need to write good tests but I need to fetch some good values for that. I'm unavailable for the rest of the week so it will need to wait.

@picnixz
picnixz force-pushed the fix/decimal/infinite-prec-140036 branch 2 times, most recently from ecefabe to 9377ab6 Compare October 26, 2025 12:43
@picnixz

picnixz commented Oct 26, 2025 •

Copy link
Copy Markdown
Member Author

@tim-one I implemented your suggestion for the digits computation when not relyin on str(q) but I kept our original implementation when we anyway need to compute str(len(q)) in the end.

I agree that the a * _LOG_10_BASE_2_LO ignores the lower-bits if a is expressed with more than 53 bits. However, how would you suggest that we fix this? note that it's fine to have an imprecise value of _LOG_10_BASE_2_LO as soon as the latter is smaller than the real value of log10(2).

@picnixz
picnixz marked this pull request as ready for review November 7, 2025 09:31
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/test/test_decimal.py
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/_pydecimal.py Outdated
Comment thread Lib/_pydecimal.py Outdated
picnixz and others added 2 commits November 9, 2025 18:14
Co-authored-by: Mikhail Efimov <efimov.mikhail@gmail.com>
@picnixz
picnixz requested a review from tim-one November 9, 2025 17:31
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 30, 2026
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Jun 8, 2026
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 90 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Sep 10, 2026
@picnixz picnixz added sprint and removed stale Stale PR or inactive for long period of time. labels Oct 21, 2026
@github-project-automation github-project-automation Bot moved this to Todo in Sprint Oct 21, 2026
@github-project-automation github-project-automation Bot moved this from Todo to In Progress in Sprint Oct 21, 2026
@picnixz
picnixz merged commit f3c27a4 into python:main Oct 23, 2026
57 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Sprint Oct 23, 2026
@picnixz
picnixz deleted the fix/decimal/infinite-prec-140036 branch October 23, 2026 17:10
@picnixz picnixz added needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Oct 23, 2026
@miss-islington-app

Copy link
Copy Markdown

Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Oct 23, 2026

Copy link
Copy Markdown

GH-160444 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Oct 23, 2026
@bedevere-app

bedevere-app Bot commented Oct 23, 2026

Copy link
Copy Markdown

GH-160445 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 23, 2026
picnixz added a commit that referenced this pull request Oct 23, 2026
…sion (GH-140044) (#160445)

gh-140036: _pydecimal: avoid slow exponentiation in floor division (GH-140044)
(cherry picked from commit f3c27a4)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz added a commit that referenced this pull request Oct 23, 2026
…sion (GH-140044) (#160444)

gh-140036: _pydecimal: avoid slow exponentiation in floor division (GH-140044)
(cherry picked from commit f3c27a4)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants